Repository navigation
Conversation
The session screen could paint the loading skeleton and a Connection lost line in the same frame, with the last skeleton bubble sliced by the row, and the load-error card restated the same failure again beside Send. A lost agent transport now owns the body: the SDK's own connection-lost code resolves to a retryable terminal failure with the existing connection-trouble copy and Retry, whatever type the status indicator carries, and an exhausted app-wide transport leg does the same while the open still has nothing to show. While the body states a failure, the fixed footer row and the composer reason stay silent, so one failure is stated once with the Retry that resolves it. The skeleton's bubbles can shrink instead of overflowing a body shorter than they are.
|
bot: Device proof on the Android emulator against the local dev stack, with the session-ingest worker stopped so the session open fails. Before the change the load-error card and the composer reason state the same failure twice; after the change the card states it once with its Retry and no footer row. Design gate contact sheet for the four standard cells (0 errors, 4 pre-existing touch-target warnings). |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe incremental commit reads the composer's font scale from Assumptions: Files Reviewed (2 files)
Previous Review Summaries (13 snapshots, latest commit 557fd81)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 557fd81)Status: 2 Issues Found | Recommendation: Address before merge Executive SummaryThe composer slot now follows the font-scaled field height, but it reads the font scale from RN's cached dimensions instead of the app's live scale, and still omits the field wrapper's 2 pt border, so the slot can still disagree with the composer it reserves. The bubble/skeleton placeholder from the prior commit is unchanged and remains correct. Assumptions: Overview
Issue Details (click to expand)WARNING
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous review (commit 09c5885)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe incremental commit replaces the composer placeholder's rem scale steps with platform point sizes, resolving the previous gap at the default scale, but the field still reserves a fixed 44 pt while the real composer field grows with the system font scale. Assumptions: Overview
Issue Details (click to expand)WARNING
Files Reviewed (2 files)
Resolved since the previous review: the prior Fix these issues in Kilo Cloud Previous review (commit 711243f)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe new composer-shaped loading slot still stands ~6–7.5 dp shorter than Assumptions: Overview
Issue Details (click to expand)WARNING
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous review (commit 416d917)Status: No Issues Found | Recommendation: Merge Executive SummaryThe incremental commit chooses the reserved bottom slot by the session's resolved kind and adds the composer placeholder for writable sessions, while preserving the read-only footer placeholder; no correctness, security, or performance defect was found in the changed files. Assumptions: Files Reviewed (8 files)
Previous review (commit 2581362)Status: No Issues Found | Recommendation: Merge Executive SummaryThe incremental commit replaces the fixed-width Assumptions: Files Reviewed (4 files)
Previous review (commit 81d14cf)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe incremental commit paints the loading value slot as one bar; its only new concern is that the fixed Assumptions: Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (4 files)
Previous review (commit 630f55d)Status: No Issues Found | Recommendation: Merge Executive SummaryThe only change since the last review removes the Assumptions: Files Reviewed (1 file)
Previous review (commit 924a3c1)Status: No Issues Found | Recommendation: Merge Files Reviewed (4 files)
Previous review (commit cd4741e)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe incremental commit Assumptions: Overview
Issue Details (click to expand)WARNING
Files Reviewed (6 files)
The earlier Previous review (commit 14f4a7f)Status: No Issues Found | Recommendation: Merge Executive SummaryThe incremental commit Assumptions: Files Reviewed (6 files)
Scope note: the incremental commit also adds the pill's dash slots and Previous review (commit bd993bf)Status: No Issues Found | Recommendation: Merge Executive SummaryThe incremental commit Assumptions: Files Reviewed (3 files)
Resolved since the previous review: the shared Previous review (commit 1cd5ee4)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe skeleton and app-wide Retry fixes land correctly, but the new Assumptions: Overview
Issue Details (click to expand)WARNING
Files Reviewed (5 files)
Fix these issues in Kilo Cloud Previous review (commit 06e4594)Status: 2 Issues Found | Recommendation: Address before merge Executive SummaryThe session-state consolidation is sound, but the skeleton Assumptions: Overview
Issue Details (click to expand)WARNING
Files Reviewed (5 files)
Reviewed by deepseek-flash-latest · Input: 88.4K · Output: 7.2K · Cached: 489.5K Review guidance: REVIEW.md from base branch |
Two review findings on the lost-connection state. The skeleton rows were the flex items that overflowed, and each bubble sat in an auto-height wrapper, so flexShrink on the bubble could not reduce anything. The bubbles are now the list's own flex items, each with its margin and alignment, and the container clips, so a body shorter than the placeholder compresses the shapes instead of letting one be sliced. A spent reconnect budget is only cleared by resetting the shared transport leg, so the terminal error now carries which recovery its Retry runs: reopening the session for a failure the session reported, and the connection retry for a lost leg.
|
bot: Re-ran the design gate at the new head and added the placeholder capture.
|
The recovery a lost transport's Retry runs depends on which transport it was: a session whose own transport dropped reopens, while the shared app-wide leg needs its spent reconnect budget reset. Pass the action per branch instead of returning the connection retry for both.
While a session transcript was still loading, the screen painted an editable Message composer at the bottom. The open's transport has not answered in that window, so a read-only session replaced that composer with the read-only footer as soon as it resolved: one screen showing writing controls it did not have, then a jump to the state it did have. The bottom slot is now the session's own. Until the transcript resolves — and while no failure, agent status or pending ask means the session has already spoken — the slot holds the read-only footer's area, at the footer's own geometry, so the footer lands where the placeholder was. The composer keeps its place behind blocking cards and failures. The header context pill showed a bare ring in that same window, then grew its value when usage arrived. A session that has not reported usage now keeps both value slots, marked with the dash, so the pill fills the slots it already showed instead of changing shape late.
|
bot: Design gate and device frames re-run at head
Screen text and element bounds below are read from a |
|
bot: The automatic design review hands this pull request to the owner at head b5b761e.
|
The pending open reserved the read-only footer's area for every session, so a writable session held a 97.9 dp footer block and then swapped to its composer when the open resolved. Choose the reserve from the session's own kind: a read-only session keeps the footer's area (which lands on it unchanged), and every other session keeps the composer's, using the composer skeleton the route already paints before the metadata arrives. Both are placeholders, so neither offers a control, and the composer now lands on the geometry its skeleton holds.
The slot that stands in for the composer now mirrors the composer's input row: the field's 44 dp box and the two trailing controls at their tap-target size in the row's own gap, instead of one bar. Come with the placeholder's fill, so the loading frame reads as the composer landing rather than as a different row.
|
bot: Pending-open frames for both halves of the bottom slot, captured on the emulator at the default text size and density against the local stack, with the transcript read held so the loading window stays open.
Attachments: the two pending frames, then the gate's contact sheet. |
|
bot: Answering the two findings of the design handoff at head
Design gate at the new head: 4 cells, 0 errors, 4 warnings (one |
The placeholder row used scale steps, and this app's rem is 14 pt: `h-11` paints 38.5 pt where the field is 44 pt and the two controls are 48 pt on Android and 44 pt on iOS, so the slot fell 6 pt short of the row it replaced and the transcript moved when the composer landed. Write the sizes in points and pin them, with the rem trap asserted so a scale step cannot come back.
|
bot: Two changes since the last run, both from the review.
Attachments: the pending frame at this head, then the gate's contact sheet. |
The slot pinned its field at 44 pt, but the composer's field is its input's line box times the system font scale plus the input's vertical padding, so a larger text size made the composer taller than the slot that stood in for it. The field takes the same rule from the window's font scale; the two controls keep their fixed tap-target size. The suite pins the height at scale 1 and at a larger scale.
|
bot: Final run for this head, after the font-scale fix.
The field's box now follows the composer's own rule (its input line box times the system font scale plus the input's vertical padding), so it is 44 pt at the default text size and grows with a larger one instead of staying fixed. Attachments: the pending frame at this head, then the gate's contact sheet. |
…poser slot Two ways the slot still disagreed with the composer it stands in for: it read the window's font scale, which goes stale when the text size changes without a remount, where the composer reads the app's live one; and it left out the field's 1 pt border on each side. Take the live scale and the border, and pin both.
|
bot: Re-ran both design-gate runs at this head after the live-font-scale and field-border fixes.
Attachments: the pending frame at this head, then the gate's contact sheet. |
|
bot: Answering the coverage note in the kilobot summary at this head, as a comment on the pull request because it arrived in the review body rather than in a thread. The note is accurate as a coverage observation, and it is not a defect in the shipped behaviour:
The reason it stays a note rather than a commit on this head: the evidence for this slot is a frame measured on the device at this exact commit, and the checks and the review history for it are anchored there too. Making the two mocked hooks disagree is worth doing as test hardening, and it belongs with the next change that touches this placeholder, so it lands on top of a code path that is already verified instead of moving that device frame, the design-gate run and the review for a test-only edit. |
|
bot: Re-ran the design gate at this head and its merge base with one staging, so the two runs can be attributed to each other.
Both runs and the merge-base folder are uploaded for this head. The attached sheet is this run's contact sheet. |



















Evidence
Evidence: demonstrated — on emulator-5554 at the default text size and density against the local dev stack, with the transcript read held open so the loading window stays measurable: a writable open's loading slot painted the read-only footer's reserve at the previous head (a 17.5 dp note bar at y 2112–2152 px and a 48 dp action bar at y 2184–2308 px in the footer's own gray, inside the band that starts at y 2080 px) and paints the composer's own row at this head (the field's box and the two trailing controls in the placeholder fill on the page background, with no footer gray anywhere in the frame — the controls standing 124 px, 48 pt at the device's 2.625 density); a read-only session keeps the footer's area and its footer lands on the same frame (
[0, 2080][1080, 2337]px); the reported frame itself is covered by the mounted suite, where a stalled open with a connection-lost indicator now renders one retryable card with its Retry instead of the skeleton and aConnection lostline, for indicator typeserror,info,progressandwarning.Problem
The session screen had no single owner for a lost agent transport:
resolveSessionTerminalErroronly treated a status indicator of typeerroras the body's failure, and the SDK's own lost-transport code (agent-connection-lost) carried catalog copy instead of a class, so the classifier called itunknown: copy written for an unknown load failure and no Retry.Connection lostin the same frame — a loading state and a failure on one screen.The session could not be loaded. Retry first.beside Send), so one failure was stated twice with one meaning.Five defects of the same window survived that change, and the design review named them across its successive reviews:
isReadOnly. A writable open (the common case, and the one whose composer is what lands) held the footer's 97.9 dp block and then swapped to its ~56 dp composer, so the slot jumped by the difference and briefly described the wrong footer.mutedequals thesecondaryfill of the footer the area stands in for, so the two bars painted the container's own colour.Change
apps/mobile/src/components/agents/session-terminal-error.tsCouldn't load this session, messageConnection trouble. Please retry in a moment., Retry. No new copy; both strings already exist in every mobile locale.isRetryableTerminalFailureanswers true for that code, so the other surfaces agree.connectionLostfact for the app-wide transport leg, which counts only while the open still has nothing to show.apps/mobile/src/components/agents/session-detail-content.tsxisReadOnly(the kind the session resolved to), so neither case jumps when the open resolves. Both reserves are placeholders, so neither offers a control. A failure, an agent status or a pending ask is the session speaking and keeps the composer it already has.apps/mobile/src/components/agents/session-detail-skeleton.tsxSessionReadOnlyFooterPlaceholderholds the read-only footer's area — its container, a line and a button bar at the footer's own child sizes — so the footer lands on its own geometry instead of replacing a composer.bg-muted-soft, the one gray that differs from thebg-secondaryfill the area reserves, so the reserved area reads as loading instead of as an empty block.h-11/h-12painted 38.5 pt and 42 pt against the field's 44 pt andCONTROL_HIT_TARGET's 48 pt on Android (44 on iOS) and the field follows the composer's own height rule (its input line box times the live system font scale, plus the input's vertical padding and the field's 1 pt border on each side) so a larger text size neither clips it nor leaves a gap, andSessionComposerSkeletonmirrors the composer's own input row — the field's 44 dp box and the two trailing controls at their tap-target size in the row's gap — instead of one bar, so the composer lands on the row the placeholder held. The route paints the same component before the session's metadata arrives.main: the placeholder takesmain's removal of its exit fade (the transcript takes its place in the same commit, so no snapshot paints over the real rows) and keeps this branch's bubble geometry — the bubbles stay the list's flex items, so a body shorter than they are compresses them and clips at its own edge instead of painting over the row below it.apps/mobile/src/components/agents/context-usage-display.tsloading, which the screen paints as one bar over that row; after the change a dash beside a bar no longer reads as half-painted content.apps/mobile/src/components/agents/session-context-metrics.tsxbg-muted-softbar fills a full-bleed box over it, so the placeholder takes the settled row's own text metrics at any font scale instead of a width of its own.session-terminal-error.test.tscovers the lost code on every indicator type and the app-wide leg, plus the loaded-transcript exemption;session-detail-content.test.tscovers the reported frame (skeleton replaced by one retryable card, no footer line, no send reason), the rewritten footer expectations, and the bottom slot (held area with no composer, then the read-only footer on that same frame);context-usage-display.test.tsandsession-context-metrics.test.tscover the dash-marked slots and the single loading bar that replaces the dash while the row loads;session-detail-skeleton.test.tspins the reserved footer's geometry and the fill that stands off it, and fails if a bar goes back to the container's own colour.Verification
mainatb5b761e5b; the conflicts incontext-usage-display.tsandsession-context-metrics.tsxkeep both sides —main's badge formatter and left-to-right cost run, and this branch's cost-slot model — and the suite that pins the direction now stubs the pill's skeleton, so it loads in the pure project without the animation package.pnpm --filter kilo-app exec vitest run src/components/agents/at this head: 296 files, 5317 tests pass.pnpm --filter kilo-app run typecheck: clean after the tRPC and harness SDK builds. oxlint (apps/mobile/.oxlintrc.json) and oxfmt: clean on every changed file.ses_00000000000LiveCliStream01:2b4f203d3c: the open's loading frame, in all four design-gate cells, paints the Message composer with mic and send and a 51.0 dp context ring; the session then settles read-only with theThis is a read-only sessionfooter and Continue, and the composer is replaced by it.14f4a7f594: the same loading frame paints the reserved footer area (no composer) and a 78.1 dp pill with both value slots, but the reserved footer's two bars and the pill's second dash are indistinguishable from their own surfaces.630f55d95: with the API held for 20 s so the loading window can be captured frame by frame, the reserved footer paints two bars — 46 px of note line and 126 px of button floor — and the pill holds a 73 px cost bar at the baseline of its dash. With the API refused, the same screen settles into the retryable failure with the dash in that same slot, and the pill keeps its width (97.1 dp / 97.5 dp / 97.9 dp; frame measurements in## Evidence).2581362723: the loading frame paints no dash. In all four cells the value row is 40.0 dp and the bar fills it —[346.3, 93.0, 40.0, 7.2]inside the row at[346.3, 89.5, 40.0, 14.1], vertically centred, mirrored to[25.5, 93.0]in the Arabic cell. The pill is 97.9 dp while loading and 97.1 dp once the open settles with its cost (—8.4 dp at[910, 235],$0.4027.4 dp at[941, 235]), where the merge base's same frame is a 51.0 dp ring.eae9f3804: with the app's data cleared so the open has nothing cached, the app-wide transport leg exhausted (the ingest socket unreachable) and the HTTP API held so the open stays unresolved, all four cells paint the placeholder and no composer. The skeleton body ends at y 792.4 dp, its last bubble[14.1, 732.6, 255.6, 48.8]11.1 dp above that edge, andSessionReadOnlyFooterPlaceholderoccupies[0, 792.4, 411.4, 97.9]with its 17.5 dp note bar and 48 dp button floor.0a1a74b71, same staging and same cleared app: the same three bubbles at the same offsets, but the body ends at y 824.4 dp on theChatComposerand no placeholder is painted — the composer inside the loading window this change removes.b5b761e5b, one staging for both: the session's transcript socket stalled while its metadata arrived. The merge basee38da3eabpaints the failure card with Retry, a secondThe session could not be loaded. Retry first.line under it, and the composer; this head paints the same card with Retry and no second line. In the loading window the base paints the Message composer where this head paints the reserved footer area — its note bar at[356, 2110, 724, 2156]and its button bar[37, 2184, 1043, 2310](48 dp) inside the footer's own container[0, 2080, 1080, 2337]— and the header pill paints no value text, only the bar over the row.282ec6c01, writable open with the transcript read held: the slot paints the composer row (field box plus two controls) in the placeholder fill, the controls 124 px tall; the same staging at headb5b761e5bpaints the read-only footer's two bars inbg-muted-softinstead, and both frames are attached in the comments that report these runs.282ec6c01, cloud-agent session with no readable transcript: the same slot holds the composer row while the read is in flight, then the retryable card with its Retry takes the body.Connection lostline; after it, neither — the retryable card with its Retry instead. Covered for indicator typeserror,info,progressandwarning, and for an exhausted app-wide transport leg.282ec6c01, on the agent chat session route for the dev fixture session: 4 cells, 0 errors, 4 warnings — oneunsettledwarning per cell, because the fixture session streams while the cells are measured (11–13 s against the gate's 8 s settle window), not a layout finding. Nothing this change paints is flagged: no clipped element, no layout shift, no touch target, no safe area and no RTL ordering finding.e38da3eab(this branch's merge base onmain), restarting Metro and reloading the app: 4 cells, 0 errors, 8 warnings — the same fourunsettledones, plus the composer voice control's touch target in all four cells, which the head's captured frame does not paint. Nothing this change paints is flagged in either run.Limitations
The exact dark cell from the reporting run also needs the socket to close while the transcript fetch is still pending. In the local stack both ride port 8800, so a stopped worker fails the fetch first (the error-atom state) and a dropped port hangs both: that split cannot be staged on this VM. The indicator path is therefore proved in the mounted suite, and the device run covers the loading, the read-only and the load-error states.
The design gate's default observation window closes the cell before this fixture's transcript settles, so the runs above show either the first painted frame or the loading frame; the settled read-only frame was measured on the same device with a longer observation window (
--observe-ms 7000), which also reports the transcript's own scroll-in as layout-shift errors on the merge base and on this branch alike. The loading frame in the runs above is the gate's own capture right after the link opens, while this fixture's transcript is still unresolved; an earlier head held that window open with a delayed API instead, and that capture also showed the app's own slow-load card once the delay outlasted its threshold.A writable pending window was also staged on the device by holding one read (
cliSessionsV2.getSessionMessagesPage) for 9 s. That makes the window reproducible and is not the app's own timing: past ~20 s the read's client deadline turns the open into the retryable card instead. While a session's kind is still unresolved (metadata in flight) the slot is the composer's, so a read-only session passes through the composer row before its footer; both are placeholders and neither offers a control.Production prerequisites
None.
Proxy answers
The owner's proxy gave these answers in the owner's place.
Answer: Both: retryable error for a lost transport AND a non-clipping skeleton for ordinary loading
Confidence: 0.55 · Precedent: Fix the root cause and the class of the issue: "if anything repeats the same mistake over and over again, fix the class" (00eb2a1d3e4b)
Answer: Yes: transient class, existing 'Connection trouble' copy, Retry present
Confidence: 0.80 · Precedent: Reuse as much as possible of the current implementation: "reuse as much as possible from the current impl" (0a2b68365a4b); plans cover the retryable-error state (df491946927a)
Answer: Suppress both while the body states the failure
Confidence: 0.70 · Precedent: Remove redundant copy and keep surfaces consistent: "go make the public dashboard a delight to use" (0e57ea66f95b); best-in-class mobile UX (00eb2a1d3e4b)
Answer: Yes, while the transcript is still loading
Confidence: 0.60 · Precedent: Fix the class on every affected variant: "fix this for all shared services model pickers on all surfaces" (75f8833fe8c1)
Answer: Fix: hide the dash while loading, one bar spanning the 40.2 dp value row
Confidence: 0.85 · Precedent: UX rule 'Stop mobile design and UX defects before they land, with a design gate': session 6341f41c09ab, owner wrote 'I very often have design/ux fixes sessions - I don't want to have those'. Also, pushing after an approval is fine: session df491946927a, 'you can push after an approval without issue'.
isReadOnlyFooterPlaceholderVisible = isSessionStatePending && !hasBlockingInteraction && !isReadOnlyTranscript, andisReadOnlyTranscript = isReadOnly && messages.length > 0. So a writable session (the common case; the composer is what lands) paints the footer's 97.9 dp block and then swaps to the ~56 dp composer, while a read-only session (the gate's CLI-stream fixture link) lands on that block at the identical frame. The critic read the base run's loading frame (a composer) and called the reserve a composer skeleton. How should the reserve pick its shape?Answer: Pick the reserved shape from isReadOnly
Confidence: 0.80 · Precedent: UX and design quality: 'no layout shift ... fix the whole class of each defect' (session 00eb2a1d3e4b, "CLS is a massive enemy"); reuse existing components (session 0a2b68365a4b, "reuse as much as possible from the current impl")
--mutedequals--secondaryin both palettes (light #f0eee6/#f0eee6, dark #1f1f24/#1f1f24, pinned by guard tests in three other components), and the reserved area paints the footer's ownbg-secondary, so bars in the bubbles' token paint the footer's fill exactly — the blank tinted block this branch already fixed once.bg-muted-softis the house token for a bar on a secondary surface (profile credits, hub card, PR review checks, the header pill). Its bar is also lighter than the filled Continue button it reserves. How do you want it answered?Answer: Keep bg-muted-soft, state the reason in the body
Confidence: 0.72 · Precedent: Keep similar elements consistent across surfaces and improve within the design system (session df491946927a, accepted "Improve within the design system"); simplest design (session a30dc8dc131a, "Simplest please")